fix(fonts): supplement alias faces from the canonical family - #3085

Reproduced the bug against live Google Fonts, confirmed this branch fixes it, and found one thing I'd like addressed before it lands.

Repro (main, unfixed)

font-family: Helvetica, upright + italic text, real network:

google css2 family= queried: [ "Helvetica" ]
  Helvetica  normal 400  EMBEDDED INTER BUNDLE
  Helvetica  normal 700  EMBEDDED INTER BUNDLE
  Helvetica  normal 900  EMBEDDED INTER BUNDLE
  Helvetica  italic 400  FETCHED FROM GOOGLE
  Helvetica  italic 700  FETCHED FROM GOOGLE

Rendered headless, the upright line is Inter and the italic line is Google's Helvetica substitute. Two typefaces under one font-family, exactly as described. css2?family=Helvetica returns 200 today, so the "alias names 4xx" comment is indeed stale.

With this branch

google css2 family= queried: [ "Inter" ]
  ... italic 400 / 700 now come from Inter

Italic renders as Inter Italic. Bug is gone, and weights 100-800 that previously had no face at all now resolve to real Inter weights. The fix is at the right layer, in the one function every aliased family routes through, and reusing CANONICAL_FONT_DISPLAY_NAMES is the right call: all 18 canonical slugs have an entry and every one is a real Google family.

Blocking: the payload multiplies

Google serves Inter as a variable font, so css2 hands back the same woff2 URL for every static weight. The supplement loop embeds that identical base64 blob once per weight.

One family, Helvetica, compiled HTML:

Six @font-face rules (100/200/300/500/600/800) carry byte-identical copies of one 25 KB blob; the two italics are another duplicate pair.

It compounds across aliases, because names that used to 4xx now fetch. Four families that all resolve to Inter (Helvetica, Arial, SF Pro, Verdana):

963 KB carrying five distinct fonts. The duplication is pre-existing in the supplement loop, but this PR is what makes it fire on every cross-typeface alias, so I'd rather not land the amplification untouched.

Smallest fix that keeps semantics: when consecutive Google faces share a src, emit one rule with a weight range (font-weight: 100 800) instead of one rule per weight. That is the correct declaration for a variable font anyway and takes the 4-family case back under 300 KB. If you'd rather keep this PR to the one-line correctness fix, that's fine by me, but please open the follow-up and link it here.

Non-blocking

1. ?? originalCaseFamily is unreachable today, and if it ever becomes reachable it silently restores this exact bug. Worth a line in deterministicFonts.test.ts asserting every CANONICAL_FONTS key has a CANONICAL_FONT_DISPLAY_NAMES entry, then the fallback can go.
2. resolveAliasDisplayName() already exists in the same module and does this lookup in one call. Would drop the extra import. Pure taste, ignore if you prefer the explicit map.

Test file looks right to me. It classifies into the unit/bun lane with no manifest edit needed, and the assertions do bite: on unfixed source queriedFamilies is ["Noto Sans"], so the first test fails and the Montserrat guard passes. Matches your 1 pass / 1 fail.

R2 at 1a87dcb02. The core fix is valid: resolving the supplementary query through the canonical display name at packages/producer/src/services/deterministicFonts.ts:531-534 eliminates the real mixed-typeface output, and the positive assertions at packages/producer/src/services/deterministicFonts-aliasSupplement.test.ts:111-132 pin both the query family and injected bytes. The earlier resolver/fallback concerns are also cleanly closed.

blocker — the payload collapse does not handle Google’s real no-text= response shape. packages/producer/src/services/deterministicFonts.ts:545-557 only collapses consecutive faces with the same source/style/unicode-range. When extractGoogleFontsText exceeds its 1,700-byte budget and returns undefined (packages/producer/src/services/deterministicFonts.ts:1130-1143), Google returns the normal subset CSS ordered by weight, then unicode subset: 100/latin, 100/latin-ext, 200/latin, 200/latin-ext, etc. A variable font reuses the same source per subset across weights, but those matching faces are never adjacent, so every base64 blob is still emitted once per weight.

I reproduced this against the current code with a hermetic real-shape response: two unicode subsets × weights 100/200/300/500, one shared source per subset, and HTML large enough to omit text=. Expected four emitted blobs (100–300 + 500 for each subset); head emitted eight. The existing test at packages/producer/src/services/deterministicFonts-aliasSupplement.test.ts:135-169 has one subset only, so it cannot falsify this path.

Please group supplementary faces by (dataUri, style, unicodeRange), numerically sort each group, then partition ranges around covered embedded weights; add the interleaved-subset/no-text= case. That also makes the “Google ordering is not guaranteed” claim true structurally instead of relying on adjacency.

Exact-head unit/build/lint lanes are green and the touched suite passes 4/4 locally, but the exact-head regression workflow is also red on style-15-prod visual comparison; rerun or resolve that before the next verdict.

Verdict: REQUEST CHANGES
Reasoning: The alias correctness fix is real and well placed, but the requested payload guard still misses a normal production response path and leaves the amplification intact for larger compositions.

— Magi

Re-review at 4900b2b3, additive to Magi's two rounds rather than a fresh pass. Magi's R2 is pinned to 1a87dcb0, which is two commits stale, so the useful questions are (a) is the R2 blocker actually closed at this head and (b) is the red required check what the author says it is. Both answers below, and the second one is the reason I am not stamping.

Magi's R2 blocker is closed

R2 asked for grouping by (dataUri, style, unicodeRange), numeric sort, and partitioning around covered embedded weights. That is what 4900b2b3 does:

• groupFacesBySource (deterministicFonts.ts:478-487) keys on exactly [dataUri, style, unicodeRange ?? ""], so non-adjacent faces sharing a source now group. This is the specific thing the adjacency version could not do on the no-text= interleaved response.
• partitionWeightRuns (:518-538) sorts numerically before walking, and breaks a run via spansCoveredWeight (:493-510) whenever a bundled weight falls strictly inside the span.
• Emission order is preserved by firstAppearance (:593-595), which re-sorts the collapsed runs back to their position in the response. That matters for overlapping unicode-range, which resolves last-defined-first, and it is the part I would most have expected a collapse to get wrong.

Worked through the concrete case: bundle covers 400/700 normal, Google returns 100/200/300/500/600/800/900 on one source. Runs come out 100 300, 500 600, 800 900. No range spans 400 or 700, so no collapsed rule shadows a bundled face. Strict inequality in spansCoveredWeight is right rather than off-by-one, because supplementary has already filtered out every covered (weight, style), so a face's own weight can never be the covered one.

Both R1 non-blocking items are also done: resolveAliasDisplayName replaces the explicit map lookup, and the ?? originalCaseFamily fallback is gone. On the fallback specifically, the new behaviour is the safe direction. A resolution failure now skips supplementation entirely instead of silently re-querying the authored family, so the failure mode is missing weights rather than a reintroduced typeface mix. Test 5 pins it, and it pins the right map: FONT_ALIASES at :378 is FONT_ALIAS_MAP with a cast, the same object the branch reads at :554, so there is no map-divergence gap between what the test iterates and what the code dispatches on.

Both fetchGoogleFont callsites are correct at this head: :578 queries the canonical for the aliased path, :614 queries the authored name for Path 2, which is right because an unaliased family should be fetched under its own name.

The red regression check: the author's explanation is correct, and here is the proof

regression is one of the 8 required contexts on main, and it is failing at this head (regression-shards (shard-9, style-15-prod ...)). Several sibling shards read cancelled, which is fail-fast fallout and inconclusive, not red.

The author states the style-15-prod baseline encodes the bug. I checked rather than took it. Extracting every @font-face from the recorded packages/producer/tests/style-15-prod/output/compiled.html and hashing each base64 payload gives 14 faces over 8 distinct blobs:

Three separate authored families share byte-identical upright blobs, which is the embedded Inter bundle resolving correctly through the alias. The clincher is the italics: Helvetica italic and Helvetica Neue italic are different blobs from each other. Both families alias to Inter, so if either had been supplemented from the canonical they would be the same bytes. They are not, because each was fetched under its own authored name. Two typefaces under one family name, exactly as #3083 describes, recorded into the baseline.

Helvetica Neue normal 300 as its own unique blob is the same story and independently corroborates the issue's table, which lists helvetica neue → inter injecting precisely 300, 400i, 700i.

I confirmed the baseline I measured is current: git hash-object on that file is 7926829bc40225df3490ba15f077bed975d48daf, identical to the blob SHA the contents API reports for it on main.

So the failure is the fixture asserting the old, wrong output. 43/100 checkpoints diverging from around 7.7s, where the italics enter, is consistent with that and not with a broader rendering regression. The collapse logic is not implicated either way here, since every face in that fixture has a distinct source and nothing collapses.

What this needs, and why it is not the contributor's to do

The consequence is that this PR cannot go green on its own. style-15-prod has to be re-recorded against the corrected fonts, and the author has already said they lack the render infra and offered to do it given the command. That is a maintainer action, so this is blocked on us rather than on them, and it has been sitting since 10 Aug.

Worth being explicit for whoever picks that up: re-recording is not a formality here. It is accepting a deliberate visual change to that fixture, where italic text stops being Helvetica and becomes Inter Italic. That is the intended correction, but it should be an eyes-on approval of the new video, not a blind re-record.

One thing worth a maintainer's eye

The author flagged, unprompted, that font-weight: 100 300 on a genuine variable font means an intermediate weight interpolates rather than snapping to a discrete face. That is the correct declaration for a variable font and I agree with the direction, but it is a real rendering change for any composition currently requesting an off-step weight, and no test pins it. Neither review has responded to that point. I would not block on it, but it belongs in the merge note rather than only in a PR comment.

Verdict

Not approving, on a single mechanical ground: a required check is red. regression is required on main and it is failing at this head, and I do not stamp over red required CI even when I believe I know why it is red. I have said above why I think the red is a stale baseline rather than a defect, and if a maintainer re-records style-15-prod and regression goes green, I have no other blocker on this PR at 4900b2b3.

Also noting for the record that Magi's CHANGES_REQUESTED is still the live review decision and is pinned to 1a87dcb0. On my read the blocker it names is addressed at this head, but that is Magi's to clear, not mine to overrule.

Separately: this is an unusually good bug report. The 51-alias enumeration, the live measurement of which 19 actually mix today, and the point that the set will drift with Google's catalog rather than with our code are all things that made this reviewable without re-deriving the analysis.

— Rames Jusso (James's assistant)